Skip to content

Clear phpstan-baseline.neon by fixing the underlying test issues#41

Merged
turegjorup merged 7 commits into
developfrom
fix/phpstan-test-baseline-cleanup
May 12, 2026
Merged

Clear phpstan-baseline.neon by fixing the underlying test issues#41
turegjorup merged 7 commits into
developfrom
fix/phpstan-test-baseline-cleanup

Conversation

@turegjorup
Copy link
Copy Markdown
Collaborator

Summary

PR #40 added tests/ to PHPStan's paths and grandfathered 46 pre-existing level-8 issues into phpstan-baseline.neon with a note that the baseline should shrink over time. This PR pays that debt down — to zero — and removes the baseline file.

PHPStan still runs at level 8 across src and tests and remains clean.

How the 46 errors were cleared

Six small commits, each isolated and easy to review:

  1. Drop nullable from $provider test property$this->provider is set in setUp() and never assigned null; the ?OpenIdConfigurationProvider declaration was the root cause of 25 Cannot call method X() on …|null errors. Single-character fix, 25 baseline entries removed.
  2. Install phpstan/phpstan-mockery for Mockery type stubs — the official extension shipping stubs for the shouldReceive(...)->andReturn(...) fluent API. 8 entries removed.
  3. Narrow validateIdToken claim accesses via @var — annotate each call site's local $claims with an object{nonce, aud} shape so the subsequent property reads type-check. 5 entries removed.
  4. Fail loudly on missing fixtures and malformed authorization URLs — replaces silent (string) coercion of file_get_contents / parse_url (which masks false returns as empty strings) with a loadMockFixture() helper using assertNotFalse / assertIsArray, plus an assertIsString guard on parse_url. 4 entries removed.
  5. Clear remaining level-8 test issues — three small fixes: expectNotToPerformAssertions() replacing assertTrue(true) tautology; reflection-based marker-implementation check replacing constant-folded is_subclass_of tautology; ?int type on MockJWT::\$leeway. 4 entries removed.
  6. Delete now-empty phpstan-baseline.neon — and drop its include line from phpstan.neon.

Why no (string) casts for string|false returns

A silent (string) cast of false coerces to the empty string, which then flows into JSON / URL parsing and produces confusing downstream assertion failures rather than the actual diagnostic ("file not readable", "URL has no query"). Commit 4 handles this at the actual error boundary with PHPUnit assertions, so a missing fixture or malformed URL fails the test with a precise message.

Test plan

  • task test:coverage — 87 tests, 135 assertions; 100% coverage on OpenIdConfigurationProvider (24/24 methods, 148/148 lines) preserved.
  • task analyze:php — PHPStan max level 8, no errors, no baseline.
  • task lint:php — clean.
  • CI matrix on PR (PHP 8.3/8.4/8.5 × stable/lowest).

Follow-up still owed

Two pre-existing (string) casts in src/Security/OpenIdConfigurationProvider.php (lines 373 and 472) coerce mixed JSON-decoded values to string. They're not the string|false antipattern — the source is JWKS / OIDC discovery payload whose schema specifies strings — but replacing them with explicit is_string guards would catch any future malformed-payload case at the actual boundary. Will follow up separately so this PR stays scoped to the test/baseline cleanup.

🤖 Generated with Claude Code

turegjorup and others added 6 commits May 12, 2026 10:43
`$this->provider` is initialized in `setUp()` and never assigned null,
so the nullable type was misleading. Dropping the `?` removes 25 of
the 46 grandfathered errors in `phpstan-baseline.neon` — every
`Cannot call method X() on …|null` was a downstream consequence of
that one type.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`shouldReceive(...)->andReturn(...)` is the Mockery fluent API. The
return type of `shouldReceive()` is declared as
`Mockery\ExpectationInterface|Mockery\HigherOrderMessage`, and the
chained methods only exist on `ExpectationInterface` — PHPStan can't
tell the call sites are safe without type stubs.

`phpstan/phpstan-mockery` is the official PHPStan extension shipping
those stubs. Adding it removes 8 entries from `phpstan-baseline.neon`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`validateIdToken()` is declared `: object` in the library — PHPStan
can't see the dynamic `nonce` / `aud` properties of the decoded JWT
payload. Annotate each call site's local `$claims` with an
`object{nonce, aud}` shape so the subsequent property reads type-
check. Removes 5 entries from `phpstan-baseline.neon`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
`(string) file_get_contents(...)` silently coerces the `false` returned
when a fixture is missing into an empty string, which then flows into
`json_decode` and produces `null`. Tests downstream of that report
confusing assertion failures instead of "the fixture isn't there".
Similarly, `(string) parse_url(...)` masks malformed-URL failures.

Replaces the three duplicate fixture loads with a single
`loadMockFixture()` helper that uses `assertNotFalse` and
`assertIsArray` to fail the test at the actual error boundary. The
`parse_url` call adds an `assertIsString` guard for the same reason.

Removes 4 entries from `phpstan-baseline.neon`.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three small cleanups, each removing one entry from the baseline:

- `testCheckResponseSuccess` swaps `assertTrue(true)` (which PHPStan
  correctly flags as a tautology) for `expectNotToPerformAssertions()`,
  the PHPUnit idiom for "this test verifies the call doesn't throw".
- `testAbstractBaseImplementsMarker` was using a constant-folded
  `is_subclass_of(...)` call wrapped in `assertTrue`. Replaced with a
  runtime `ReflectionClass::getInterfaceNames()` check + `assertContains`
  — PHPStan can't fold the reflection result, and the assertion stays
  semantically meaningful (catches a future regression that removes
  the marker from the deprecated abstract).
- `MockJWT::$leeway` declares a type (`?int`) so the property satisfies
  PHPStan's missingType.property rule.

Baseline now empty — see follow-up commit.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The four predecessor commits cleared the 46 grandfathered errors that
`phpstan-baseline.neon` was absorbing. With the baseline at zero
entries the file serves no purpose, so drop it (and its include line
from `phpstan.neon`) rather than keep an empty alias around.

PHPStan still runs at level 8 across `src` and `tests` and remains
clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@turegjorup turegjorup self-assigned this May 12, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (4c8916e) to head (54be58b).

Additional details and impacted files
@@             Coverage Diff             @@
##             develop       #41   +/-   ##
===========================================
  Coverage     100.00%   100.00%           
  Complexity        62        62           
===========================================
  Files              1         1           
  Lines            172       172           
===========================================
  Hits             172       172           
Flag Coverage Δ
unittests 100.00% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

- `php-cs-fixer` (@symfony ruleset) flagged a redundant `@param`
  PHPDoc on a typed parameter in `getMockHttpSuccessResponse`.
  Auto-fixed.
- `composer normalize` reorders `phpstan-mockery` alphabetically
  within `require-dev` (it landed at the bottom after the manual
  `composer require`, between `phpunit/phpunit` and nothing).
- The `changelog` CI step requires every PR to touch `CHANGELOG.md`.
  Added a "Tooling" subsection to `[Unreleased]` summarising what
  this PR's six commits did (PHPStan scope extension, mockery
  extension install, baseline cleanup).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@turegjorup turegjorup merged commit 1774bc0 into develop May 12, 2026
16 checks passed
@turegjorup turegjorup deleted the fix/phpstan-test-baseline-cleanup branch May 12, 2026 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants